2021年C++项目中的十大bug：乍一看都正确的代码，实则暗藏玄机！
在程序员的新年祝福中，大家或多或少会来一句，新年编码无Bug、Bug越写越少……对程序员来说，无论何时何地，绕不去的话题里总有一个是关于Bug的。这不，本文作者便总结了2021年C++代码中的TOP 10 bug，或许与你有些许共通之处。
第10名：循环中的输入错误
V533 [CWE-691]（https://pvs-studio.com/en/docs/warnings/v533/） for循环内的递增变量错误，请检查i。
voidgsk_vulkan_image_upload_regions (GskVulkanImage *self, GskVulkanUploader *uploader, guint num_regions, GskImageRegion *regions){ .... for (int i = 0; i < num_regions; i++) { m = mem + offset; if (regions[i].stride == regions[i].width * 4) { memcpy (m, regions[i].data, regions[i].stride * regions[i].height); } else { for (gsize r = 0; r < regions[i].height; i++) // <= memcpy (m + r * regions[i].width * 4, regions[i].data + r * regions[i].stride, regions[i].width * 4); } .... } ....}注意内层循环中递增的是变量i，而不是r。无需多说，这个错误太经典了。
第9名：突然变成了HTML！
V735（https://pvs-studio.com/en/docs/warnings/v735/） HTML错误。"</body>"之前缺少"</ul>"标签。
QString QPixelTool::aboutText() const{ const QList<QScreen *> screens = QGuiApplication::screens(); const QScreen *windowScreen = windowHandle()->screen();
QString result; QTextStream str(&result); str << "<html></head><body><h2>Qt Pixeltool</h2><p>Qt " << QT_VERSION_STR << "</p><p>Copyright (C) 2017 The Qt Company Ltd.</p><h3>Screens</h3><ul>"; for (const QScreen *screen : screens) str << "<li>" << (screen == windowScreen ? "* " : " ") << screen << "</li>"; str << "<ul></body></html>"; return result;} 我们提供的检查不仅会检查代码，还会检查字符串中的异常。上述代码就触发了这个检查。这种情况很罕见。
作者想要创建一个列表，但添加了两个<ul>标签，导致了这个错误。这显然是输入错误。第一个标签开始列表，而第二个标签应该结束列表。下面是正确的代码：
str << "</ul></body></html>";第8名：不安全的宏
V634（https://pvs-studio.com/en/docs/warnings/v634/）中运算符 '*'的优先级高于'<<'。建议在此表达式中使用括号。
#define PM_EXP2(A) 1 << A
int process_val(const u_int8_t *data, u_int32_t data_len, u_int32_t *retvalue, ....){ *retvalue = 0; .... /* Now find the actual value */ for (; i < data_len; i++) { *retvalue += data[i] * PM_EXP2(8 * (data_len - i - 1)); } return(0);} 分析器警告说，在展开两个宏之后，可能会产生不正确的表达式。函数首先将一个函数乘以1，然后执行移位操作。幸运的是在这一行中 x * 1 << y 表达式与 x * (1 << y) 相等。如果左侧或右侧的宏中有 /、%、+、- 或其他优先级高于 << 的运算符，或者宏包含优先级低于 << 的运算符，表达式就无法正确求值了。宏本身和宏的参数必须放在括号中，来避免可能出现的问题。下面是正确的写法：
Define PM_EXP2(A) (1 << (A))第7名：被除数与除数相混淆
V1064（https://pvs-studio.com/en/docs/warnings/v1064/）整数除法的操作数'gPEClockFrequencyInfo.bus_clock_rate_hz'小于'gPEClockFrequencyInfo.dec_clock_rate_hz'。结果始终为0。
voidpe_identify_machine(__unused boot_args *args){ .... // Start with default values. gPEClockFrequencyInfo.timebase_frequency_hz = 1000000000; gPEClockFrequencyInfo.bus_frequency_hz = 100000000; .... gPEClockFrequencyInfo.dec_clock_rate_hz = gPEClockFrequencyInfo.timebase_frequency_hz; gPEClockFrequencyInfo.bus_clock_rate_hz = gPEClockFrequencyInfo.bus_frequency_hz; .... gPEClockFrequencyInfo.bus_to_dec_rate_den = gPEClockFrequencyInfo.bus_clock_rate_hz / gPEClockFrequencyInfo.dec_clock_rate_hz;} 所有字段都是整型：
extern clock_frequency_info_t gPEClockFrequencyInfo;
struct clock_frequency_info_t { unsigned long bus_clock_rate_hz; unsigned long dec_clock_rate_hz; unsigned long bus_to_dec_rate_den; unsigned long long bus_frequency_hz; unsigned long timebase_frequency_hz; ....};在中间的赋值中，被除数 gPEClockFrequencyInfo.bus_clock_rate_hz 赋值为 100000000，而除数 gPEClockFrequencyInfo.dec_clock_rate_hz 赋值为 1000000000。本例中，除数是被除数的十倍。因为所有值都是整型，得到的结果 gPEClockFrequencyInfo.bus_to_dec_rate_den 就是0。
从名字来推测，除数和被除数写反了。
第6名：选择类型时的错误
V610（https://pvs-studio.com/en/docs/warnings/v610/）未定义的行为。请检查移位操作符'>>='。右边的操作数 ('bitpos % 64' = [0..63])大于或等于左边的操作数的位长。
// bitsperlong.h#ifdef CONFIG_64BIT#define BITS_PER_LONG 64#else#define BITS_PER_LONG 32#endif /* CONFIG_64BIT */
// bits.h/* * Create a contiguous bitmask starting at bit position @l and ending at * position @h. For example * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000. */#define __GENMASK(h, l) ....
// master.h#define I2C_MAX_ADDR GENMASK(6, 0)
// master.cstatic enum i3c_addr_slot_statusi3c_bus_get_addr_slot_status(struct i3c_bus *bus, u16 addr){ int status, bitpos = addr * 2; // <=
if (addr > I2C_MAX_ADDR) return I3C_ADDR_SLOT_RSVD;
status = bus->addrslots[bitpos / BITS_PER_LONG]; status >>= bitpos % BITS_PER_LONG; // <=
return status & I3C_ADDR_SLOT_STATUS_MASK;} 注意BITS_PER_LONG宏可以是64位的。
这段代码包含以下未定义的行为：
● 在检查后，addr变量的取值范围为[0..127]
● 如果参数 addr >= 16，那么status变量的右移位数将超过int类型包含的比特数
也许作者想要减少代码行数，所以bitpos变量的定义紧跟status。但是，作者并没有考虑到int与long不同，它在64位平台上只有32位。
要改正这个错误，应该将status变量定义成long。
第5名：模块间的分析和丢失的memset
今年，我们添加了一个重要功能：对C++项目进行模块间分析。这个功能找到了codelite项目中的以下警告。
V597（https://pvs-studio.com/en/docs/warnings/v597/）编译器可能会删掉'memset'函数调用，该函数调用会给'current'赋值。擦除私有数据应该使用memset_s()。）
// args.cextern void eFree (void *const ptr); extern void argDelete (Arguments* const current){ Assert (current != NULL); if (current->type == ARG_STRING && current->item != NULL) eFree (current->item); memset (current, 0, sizeof (Arguments)); // <= eFree (current); // <=} // routines.cextern void eFree (void *const ptr){ Assert (ptr != NULL); free (ptr);}LTO（连接时间分析）使用了memset调用。编译器通过 as-if 规则发现，eFree并没有计算出任何有用的指针相关的数据。eFree只是调用了free函数来释放内存。
没有LTO，eFree调用看上去像一个未知的外部函数，所以memset会保留。
第4名：不合理的检查和Unreal引擎
最近，我们加强了对于Unreal引擎的检查。我们来看看分析器发现了什么问题。
V547（https://pvs-studio.com/en/docs/warnings/v547/）表达式'm_trail == 0'始终未false。
std::size_t m_trail;....inline int context::execute(const char* data, std::size_t len, std::size_t& off){ .... case MSGPACK_CS_EXT_8: { uint8_t tmp; load<uint8_t>(tmp, n); m_trail = tmp + 1; if(m_trail == 0) { unpack_ext(m_user, n, m_trail, obj); int ret = push_proc(obj, off); if (ret != 0) return ret; } else { m_cs = MSGPACK_ACS_EXT_VALUE; fixed_trail_again = true; } } break; ....}我们来看看这段代码的问题。
tmp变量是uint8_t类型，它的值为8比特，即[0, 255]。作者认为tmp可以为255。在m_trail = tmp +1赋值之后，作者检查有没有发生整型溢出，因为无符号整数运算可能会导致值回到起点。因此，tmp + 1操作可能为0。
但是，分析器指出 m_trail == 0永远为false。我们来看看为什么。
首先，回忆一下std::common_type。
该行代码包含加法运算。对于值和其他类型之间的二元操作，编译器使用通常的算数转换，对tmp变量实行整数提升。因此，该表达式中的扩展为1的类型，即int。这样，即使tmp的值是255，加法运算也会产生256。int类型能够存储该结果。因此m_trail==0是无意义的。
第3名：日期处理函数中的解释错误
在本例中，我们的分析器给出了多条警告：
V547 [CWE-571] 表达式'month'始终为true。
V560 [CWE-570]（https://pvs-studio.com/en/docs/warnings/v560/） 部分条件表达式始终为false。
V547表达式'month'始终为true。
V560部分条件表达式始终为false。
首先来看看这段代码，它接收一个月份的缩写，返回整数值。
static const char qt_shortMonthNames[][4] = { "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"}; static int fromShortMonthName(QStringView monthName){ for (unsigned int i = 0; i < sizeof(qt_shortMonthNames) / sizeof(qt_shortMonthNames[0]); ++i) { if (monthName == QLatin1String(qt_shortMonthNames[i], 3)) return i + 1; } return -1;} 如果操作成功，函数会返回月份的整数（1到12的整数值）。如果月份名称不正确，则返回-1。注意该函数不可能返回0。
但是，开发人员在调用该函数时，认为它在错误情况下会返回0。下面是错误使用该函数的代码：
QDateTime QDateTime::fromString(QStringView string, Qt::DateFormat format){ .... month = fromShortMonthName(parts.at(1)); if (month) day = parts.at(2).toInt(&ok); // If failed, try day then month if (!ok || !month || !day) { month = fromShortMonthName(parts.at(2)); if (month) { QStringView dayPart = parts.at(1); if (dayPart.endsWith(u'.')) day = dayPart.chopped(1).toInt(&ok); } } ....} 这段程序永远不可能运行月份值为0的情况，只会使用错误的月份值继续运行。
第2名：不注意导致的错误
V726（https://pvs-studio.com/en/docs/warnings/v726/）试图使用'free'函数释放包含'wbuf'数组的内存，但由于'wbuf'建立在堆栈中，因此是不正确的。
template<typename T>static ALWAYS_INLINE void FormatLogMessageAndPrintW(....){ .... wchar_t wbuf[512]; wchar_t* wmessage_buf = wbuf; .... if (wmessage_buf != wbuf) { std::free(wbuf); } if (message_buf != buf) { std::free(message_buf); } ....} 这段代码触发了警告。它试图删除一个栈上分配的数组，从而引发了错误。数组的内存不是从堆上分配的， 所以没有必要调用std::free。当对象摧毁时，内存也会释放。
我认为，这个警告的原因比警告本身更危险。
第1名：不注意导致的更大的错误
我们一直在警告用户的错误。但今年，我们自己也犯了错误。
V645 'strncat'函数调用会导致'a.consoleText'缓存溢出。边界不应该包含缓存的大小，而应该包含字符数。
struct A{ char consoleText[512];}; void foo(A a){ char inputBuffer[1024]; .... strncat(a.consoleText, inputBuffer, sizeof(a.consoleText) – strlen(a.consoleText) - 5); ....} 乍一看这段代码似乎是正确的，没有未定义错误。但仔细看一下这段代码：
sizeof(a.consoleText) – strlen(a.consoleText) – 5 这个表达式可能会产生负数！例如，当strlen(a.consoleText) = 508的时候，就会产生无符号整数溢出。表达式的结果就是结果类型size_t的最大值。
参考链接：https://pvs-studio.com/en/blog/posts/cpp/0901/
《新程序员003》正式上市，50余位技术专家共同创作，云原生和数字化的开发者们的一本技术精选图书。内容既有发展趋势及方法论结构，华为、阿里、字节跳动、网易、快手、微软、亚马逊、英特尔、西门子、施耐德等30多家知名公司云原生和数字化一手实战经验！